feat: store package support by upils · Pull Request #16 · upils/chisel

upils · 2026-06-19T14:44:19Z

Have you signed the CLA?

This PR enables Chisel fetching packages from the Store API in addition to Debian archives. Store-backed packages are declared in release YAML with store: <name> and default-track: <track>, and are resolved via the store's /v2/revisions/resolve endpoint.

Channel selection: the effective track is <default-track>-<store-version> (e.g. 3.1-26.10), composed from the package's default-track and the store's release version field. Risk defaults to stable when unspecified. The slicer owns this composition; the store package is a pure API client with no release knowledge.

Security: download URLs are validated for HTTPS and exact host match against api.snapcraft.io (production) or api.staging.snapcraft.io (staging, enabled via CHISEL_BIN_STAGING). The host is resolved at Open time and fixed for the store's lifetime.

Caching: packages are cached by SHA3-384 digest, consistent with the archive's cache pattern. The digest is verified on write.

Current limitation: store packages can be fetched and cached but not yet extracted — the slicer fails with a clear error at extraction time. Manifest recording of store packages is also deferred.

Copilot

Pull request overview

This PR adds initial “store” support to Chisel by introducing a new internal/store package (Snapcraft “bin” store client with caching) and wiring it into the slicer and chisel cut execution path, along with test utilities and unit tests.

Changes:

Added internal/store package (bin-store API client, URL allowlisting, caching, logging) plus a dedicated test suite.
Extended slicer runtime options and tests to resolve/fetch store-backed packages alongside archive-backed ones (while still failing extraction for store packages).
Updated CLI wiring to open configured stores and pass them into the slicer, and enabled store logging.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
internal/testutil/store.go	Adds a `TestStore` fixture implementing the new `store.Store` interface.
internal/testutil/package.go	Extracts `TestPackage` into a shared fixture type and adds store-related fields.
internal/testutil/archive.go	Updates archive test utility to use the shared `TestPackage` type.
internal/store/suite_test.go	Adds gocheck suite bootstrap for the new store package tests.
internal/store/store.go	Implements the bin store client (info/fetch/exists) with cache integration and URL validation.
internal/store/store_test.go	Adds unit tests for URL validation, Open behavior, Info/Exists/Fetch, caching, and staging env var.
internal/store/log.go	Adds logger plumbing consistent with other internal packages.
internal/store/export_test.go	Exposes internals for tests (URL validator, env var, HTTP hooks).
internal/slicer/slicer.go	Adds store sources to slicer resolution and fetch path (extraction still unsupported).
internal/slicer/slicer_test.go	Extends slicer tests to set up store fixtures and updates expected error.
cmd/chisel/main.go	Registers store logger in the main CLI.
cmd/chisel/cmd_cut.go	Opens configured stores and passes them into the slicer run.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.

upils · 2026-06-25T10:20:48Z

+		options.Arch, err = deb.InferArch()
+	} else {
+		err = deb.ValidateArch(options.Arch)


[Note to reviewer]: The arch mapping is the same for debs and artefacts for the store so these 2 functions can be used here. However, at this point they are not really "deb" specific. And it looks surprising to import the deb package here. What about extracting these functions into a arch package?

letFunny

This looks very very good Paul. I tried to do as much of a thorough analysis as I could and I don't see anything major, just a couple of comments about backwards compatibility. Thanks!

letFunny · 2026-06-25T10:13:06Z

-		reader, info, err := src.archive.Fetch(src.pkg.RealName)
-		if err != nil {
-			return err
+			// The package metadata returned by the store is not yet recorded


It is hard for me to comment here without knowing all details so I will take this as already-discussed gospel :)

I reworked this comment a bit, the first sentence was out of place. Regarding the heuristic to build the channel: this is the current agreement but is subject to change. So I am doing it in a single place, in a simple way, with a comment so it will be easy to change if we need to.

letFunny · 2026-06-25T10:14:50Z

 type RunOptions struct {
 	Selection *setup.Selection
 	Archives  map[string]archive.Archive
+	Stores    map[string]store.Store


Per the comments in the other PR, this distinction makes sense and makes the code simpler in a way. Normally in Go you don't define generics to abstract over a type (i.e. a store) but to abstract over an operation, in this case there are clearly two:

How to fetch a package.

Record information from the package in manifest.

As we discussed in the previous PR, this was contemplated but it turned out to be much more complex AFAIK. I also see the difficulty in Go where the type system is not powerful so it is hard to model fetching where archive.fetch(name) takes only one args vs three in store.fetch(name, track, risk) for example.

We agree. I could have reworked the Archive interface to have something more generic, used for archives and stores, but in reality they are different enough that 2 different interfaces made more sense.

As I go through the complete implementation I may discover that this new interface must be tweaked.

letFunny · 2026-06-25T10:20:17Z

+			}
 			pkgSources[pkg.Name] = &pkgSourceInfo{
-				// TODO: Fill with the live store handle when store support is implemented.
+				arch:  storeHandle.Options().Arch,


I was hesitant in #15 because with only that diff it looked weird. However, it looks okay here. You can postpone the refactor for arch.

I am not sure the other PR will not get comments about this so it might be helpful to copy the idea of this diff in a comment in canonical/chisel just in case.

Good point. I added https://github.com/upils/chisel/pull/15/files#r3479609705 (and did the same on the PR submitted to the main repo)

letFunny · 2026-06-25T10:39:11Z

+	}
+
+	// Download the package.
+	err = validateDownloadURL(rev.Download.URL, s.downloadHost)


This is good to prevent upstream errors impacting security. It also makes it so that a change in the urls will require a new major version of Chisel to be released. This is a similar problem to public keys in a way; we solved that one by including it in the release itself so that the release can evolve if the keys changes and they are not hardcoded in Chisel. Is that the experience we want here?

This is a good point. I discussed it with Bowen before moving forward on this: these URLs redirect to CDN URLs, so I understand they are as likely as the API host to change (so rather unlikely). I will bring that topic up to Gustavo tomorrow as I agree this would create a need of a breaking change.

letFunny

Looks good to me Paul, let's see how it goes :D

letFunny · 2026-06-30T08:06:59Z

+// no release for the requested channel and architecture.
+func (s *binStore) resolveRevision(name, track, risk string) (*binRevision, error) {
+	if !nameExp.MatchString(name) {
+		return nil, fmt.Errorf("invalid package name %q", name)


That makes sense as a measure to sanitize requests to the store as you say. The only problem is that now there are two regexps that have to be valid against the same name which create some bugs in the duplication. Anyway, I think it is an acceptable trade-off.

letFunny · 2026-06-30T08:08:37Z

-		{"amd64", ""},
-		{"arm64", ""},
-		{"invalid", "invalid package architecture: invalid"},
+		{"Valid amd64", "amd64", ""},


Nit: This is overkill IMO

I reworked the test to make it more generic, only keeping one non-failing case and integrating the standalone TestOpenUnsupportedKind.

upils closed this Jun 22, 2026

upils reopened this Jun 22, 2026

upils force-pushed the store-download branch 2 times, most recently from 464d55a to 4ef2c6d Compare June 22, 2026 14:09

upils requested a review from Copilot June 22, 2026 15:52

Copilot started reviewing on behalf of upils June 22, 2026 15:52 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Comment thread internal/slicer/slicer.go

Comment thread internal/store/store.go Outdated

Comment thread internal/store/store.go

Comment thread internal/store/store.go Outdated

Comment thread internal/store/store.go Outdated

upils requested a review from Copilot June 22, 2026 16:22

Copilot started reviewing on behalf of upils June 22, 2026 16:23 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Comment thread internal/store/store.go

Comment thread internal/store/store.go Outdated

Comment thread internal/store/store.go Outdated

Comment thread internal/store/store.go Outdated

Comment thread internal/store/store.go Outdated

Comment thread internal/store/store.go Outdated

upils changed the title ~~feat: access stores~~ feat: store package support Jun 23, 2026

upils commented Jun 25, 2026

View reviewed changes

letFunny reviewed Jun 25, 2026

View reviewed changes

upils requested a review from letFunny June 29, 2026 14:08

letFunny reviewed Jun 30, 2026

View reviewed changes

upils added 15 commits June 30, 2026 14:08

feat: draft accessing bin store

643a52f

fix: adjust naming

46a353a

style: linting errors

dac1ec7

feat: refining

97bfd5b

fix: open all stores

88d0cc3

fix: reapply

bbea434

feat: wire to Run

0f2a6a2

fix: review

2ee8aa9

fix: improve naming and add more tests

fdce477

fix: properly build track value with store version

c728291

feat: refine download host validation

fd0534e

feat: use revisions/resolve endpoint

885c5d3

fix: raise consistency

ea50c19

fix: use RealName in extract error

0da89bb

fix: clean model and add guardrail

4fe25b1

upils added 7 commits June 30, 2026 14:10

refactor: clean useless method

2bb1b3e

fix: review

e299985

fix: review

a035161

fix: review

3cdb869

fix: review

79c437e

test: fix inconsistencies

684f809

test: refine

4a4472c

upils force-pushed the store-download branch from 1fa6690 to 4a4472c Compare June 30, 2026 12:13

Conversation

upils commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

letFunny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

upils Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

upils Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

letFunny left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

upils commented Jun 19, 2026 •

edited

Loading

upils Jun 26, 2026 •

edited

Loading

upils Jun 26, 2026 •

edited

Loading